feat: add NewBuf opt-out for the OpenAPI/TypeScript stage - #359
Merged
Conversation
proto.(*Buf).Generate runs an OpenAPI post-generation stage after buf generate: it promotes a generated Swagger document to the canonical OpenAPI path and derives TypeScript types from every openapi/*.swagger.json. That is correct for services that want TS types from their REST API, but a non-TypeScript service that owns a proto tree for gRPC stubs and also ships an unrelated openapi/ REST contract incurs it too — dropping a stray openapi/api.ts on every Sync. Add a fluent WithoutOpenAPI opt-out, mirroring the existing With* builder methods, that short-circuits the stage. buf dep update / buf generate and the generated-dir cleanup are unchanged, and the default still runs the OpenAPI pipeline. The stage is extracted verbatim into a guarded emitOpenAPIArtifacts method so the opt-out is unit-testable offline. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…test TestReaperReapsLegacyGroupWhenOwnerPidReused gated the owner spawn on wall-clock time.Now, but the reaper authenticates owners through processStartUnixSeconds, which derives the start second from /proc and truncates the kernel's clock-tick start time to whole seconds. A process launched a wall-clock second past the record's spawn second could still read back as that same second, so legacyOwnerAlive treated the recycled PID as the live owner and preserved the group — leaving it alive and failing the test on Linux CI. Spawn the owner in a loop that verifies processStartUnixSeconds reports a start second strictly greater than the record's, killing and retrying until it does. This gates on the reaper's own clock, removing the wall-clock/proc skew race. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…st message The WithoutOpenAPI opt-out documented that it skips the OpenAPI stage and leaves inputs untouched, but was silent on what happens to an artifact a prior opted-in run already generated and committed (e.g. openapi/api.ts): it lingers as a stale orphan with no guidance. Auto-deleting it on the skip path was rejected as the fix — the stage emits <base>.ts next to <base>.swagger.json, so a name-matched delete cannot tell a stale generated file from one a teammate later authored at that path, and per-Sync deletion would be silent data loss that contradicts the method's 'leaves inputs untouched' contract. The real fix is to make the one-time manual cleanup explicit in the method doc. Also reword two opt-out test failure messages that printed the observed bad value as if it were the requirement, so a failing run reads forward. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a fluent
(*Buf).WithoutOpenAPI()opt-out tocompanions/protoso a caller can skip the OpenAPI post-generation stage that runs afterbuf generate.(*Buf).Generateunconditionally, afterbuf generate, does two OpenAPI things:<dir>/openapi/api.swagger.jsonexists, moves it tostandards.OpenAPIPath.<dir>/openapi/*.swagger.json, runsswagger2openapithennpx openapi-typescriptto emit a.ts.That's correct for services that want TypeScript types from their REST API. But a non-TypeScript service that owns a
proto/tree and ships an unrelatedopenapi/REST contract now incurs it — dropping a stray.tsinto a service that has no use for it on every Sync.Change
skipOpenAPI boolfield onBufplusWithoutOpenAPI() *Buf, mirroring the existingWithGeneratedRoot/WithGeneratedDirsbuilder methods.emitOpenAPIArtifactsmethod;WithoutOpenAPIshort-circuits it and leaves anyopenapi/*.swagger.jsoninputs untouched.buf dep update/buf generateand the generated-dir cleanup are unchanged. Default behavior is unchanged — the OpenAPI pipeline still runs unless a caller opts out.Test
New offline unit tests in
companions/proto(followingproto_dependencies_test.go):WithoutOpenAPI(), given a dir containingopenapi/*.swagger.json,emitOpenAPIArtifactsleaves every swagger file byte-for-byte untouched and produces no.ts(and no intermediate.json). Anilrunner is passed deliberately: reaching the end without a panic proves the stage short-circuited before spawning any process.NewBufleaves the stage enabled by default, andWithoutOpenAPI()sets the flag and returns the receiver for chaining.The existing Docker-gated (
proto_companion_required) tests that exercise the real pipeline are unchanged.Downstream consumer
service-python-fastapi(issue #14) will switch its gRPC-server Sync toproto.NewBuf(ctx, s.Location).WithoutOpenAPI()once this lands — follow-up from codefly-dev/service-python-fastapi#15.🤖 Generated with Claude Code